-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add "figure mode" button [WIP] #767
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fedarko, this looks great. I like this idea a lot. As for the artifact that you are seeing when exporting, that's likely due to WebGL not being able to figure out which of the two polygons should go on top of each other, since both would have the same coordinate on the Z axis. I suggest using a Z a very small and random Z value so that this doesn't happen or better yet scale the true Z axis values by a large constant so that the positions are deterministic. Note you'll have to check that a Z value exists.
'top': '5px', | ||
'right': '40px' | ||
}).attr('title', 'Figure Mode').on('click', function(event) { | ||
scope.controllers.axes.updateVisibleAxes(null, 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For updating different attributes of the axes controller, I would suggest using scope.controllers.axes.fromJSON
, then you encode these attributes as an object and you should be good to go (see the unit tests for some examples).
* @extends EmperorAttributeABC | ||
* | ||
*/ | ||
ShapeController.prototype.makeEverythingACircle = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a unit test for this method?
Closes #753 and closes #604.
This PR adds a small lightbulb icon in the top-right corner of the display, directly to the left of the "More Options" button:
On clicking this button, the plot changes to use more easily-displayable-in-a-paper settings:
The exact things that this button does are:
The first three of these things required minimal extra code (as is shown in
controller.js
, each takes up one line of code -- I attribute this to Emperor's APIs being really well designed :).The fourth thing (making everything a circle), however, required more work -- and this is what I'm most uncertain about here.
This PR adds "Circle" as a selectable shape, and it seems to work pretty well in the JS interface -- but there are some artifacts in the exported screenshots, for example with the Jaccard moving pictures PCoA:
In-Emperor
Exported PNG (note the "triangle" borders within each circle, as well as the slight inconsistency of that one blue dot at the top being partially on top of orange)
Exported PNG (zoomed in at the top)
Anyway, I was thinking we may we want to be smarter about shapes for the 2D mode anyway -- and do something similar to how
DecompositionView._buildVegaSpec()
has an object set up to convert 3D shapes to "close" 2D shapes. That might require extra work due to a need to add more 2D shapes to Emperor, so before doing that I wanted to check to see what folks would prefer (we could also conceivably not change the shapes at all, and/or leave doing that to a separate PR). So this is marked as a draft for now.(Also, this PR should have tests, at least for things like
makeEverythingACircle()
.)